Skip to content

feat(plugin): Add Plugin evaluation support across all tiers - #28

Open
chrisknvidia wants to merge 23 commits into
mainfrom
naren/plugin-evaluation-all-tiers
Open

chrisknvidia wants to merge 23 commits into
mainfrom
naren/plugin-evaluation-all-tiers

Conversation

@chrisknvidia

Copy link
Copy Markdown
Collaborator

Restoration note

This PR replaces #17, which GitHub permanently closed after the default-branch history was consolidated and the original head branch was deleted and recreated. The original discussion and commit history remain available on #17.

This replacement contains the intended plugin-evaluation change set rebased onto the current main as one Narendran commit. The older result_display.py delta was not reapplied because current main already contains the newer, stronger hardened rendering behavior; no plugin functionality was dropped.

Summary

  • adds plugin evaluation support across Tier 1, Tier 2, and Tier 3
  • hardens plugin manifest discovery against broken or out-of-root symlinks
  • requires real integration evidence before Integration-only Tier 3 runs; both safely falls back to effectiveness and records why
  • preserves direct manifest-path CLI behavior and keeps plugin provenance and reporting explicit

Public adaptations and exclusions

  • supports public GitHub/generic Git references and same-repository offline resolution only
  • adds no internal GitLab, P4, managed execution, private provider, endpoint, credential, telemetry, or server behavior
  • Tier 2 remains local and offline; no remote vector or catalog integration is added
  • plugin MCP declarations remain statically validated and fail closed before staging
  • current dependency-remediation changes from main remain intact

Verification

  • uv run pytest -q: 3,154 passed, 7 skipped, 3 deselected
  • focused plugin schema, deduplication, lifecycle, CLI, and Tier 3 evidence suite: 149 passed
  • uv run ruff check: passed
  • OSS source and built-package boundary scans: passed
  • wheel and sdist build: passed
  • twine check --strict: passed for both distributions

A live remote Tier 3 Harbor/plugin evaluation was not run during reconstruction; the verified scope is the complete local suite, focused integration tests, packaging, and static boundary checks.

This PR is intentionally not merged.

Restores and rebases the plugin evaluation work from PR #17 onto the rewritten main history.

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
@rng1995
rng1995 force-pushed the naren/plugin-evaluation-all-tiers branch from 3a51f76 to a4a5e63 Compare August 4, 2026 19:14
chrisknvidia and others added 13 commits August 4, 2026 12:18
Use boundary-aware checks for reserved names, description heuristics, API and error documentation, MCP guidance, time references, README mentions, and nested Markdown links. Add regression coverage for #30.

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The plugin evaluation implementation and its follow-up hardening are well covered, but I found one small diff-hygiene issue to clean up.

Comment thread src/skillevaluator/utils/structured_data.py Outdated
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
rng1995
rng1995 previously approved these changes Aug 5, 2026

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after the EOF diff-hygiene issue was fixed in 3965061, the review thread was resolved, and the complete GitHub check matrix passed.

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fresh review found two actionable security issues in the plugin evaluation staging path. The previous EOF-hygiene thread is already fixed and resolved. I will address these findings while updating the branch from current main.

Comment thread src/skillevaluator/tier3/plugin_eval.py Outdated
Comment thread src/skillevaluator/tier3/plugin_eval.py Outdated
…on-all-tiers

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

# Conflicts:
#	docs/tier2-deduplication.mdx
#	src/skillevaluator/deduplication/utils/skill_collector.py
#	src/skillevaluator/embedding/extractor.py
…on-all-tiers

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
…on-all-tiers

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

# Conflicts:
#	.github/workflows/ci.yml
#	CHANGELOG.md
#	src/skillevaluator/tier3/harbor/adapter.py
#	src/skillevaluator/tier3/harbor/runner.py
#	src/skillevaluator/utils/secure_fs.py
#	tests/test_harbor_input_staging.py
Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
rng1995
rng1995 previously approved these changes Aug 12, 2026

@rng1995 rng1995 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved after fresh security review and remediation: all review threads are resolved, the branch is conflict-free against current main, the complete local suite passes (3,999 passed, 21 skipped, 3 deselected), and all 15 GitHub checks pass including Windows, packaging, DCO, and security scans.

…on-all-tiers

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

# Conflicts:
#	CHANGELOG.md
#	src/skillevaluator/cli.py
#	tests/validators/test_policy.py
…on-all-tiers

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>

# Conflicts:
#	CHANGELOG.md
#	docs/tier1-validation.mdx
#	src/skillevaluator/models/skill.py
#	src/skillevaluator/validators/quality_score.py
#	tests/validators/test_quality_score.py

@mohgupta-ship-it mohgupta-ship-it left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Posted by Codex on behalf of Mohit.

Request changes: plugin provenance is persisted through a symlink-following path after the long-running evaluation. This is a medium output-integrity risk for a same-privilege actor able to modify the selected results location. The inline note describes a no-follow, atomic remediation and the regression coverage needed.

Comment thread src/skillevaluator/tier3/plugin_eval.py Outdated

@mohgupta-ship-it mohgupta-ship-it left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Codex review on behalf of Mohit — REQUEST CHANGES

Critical sandbox/path-integrity blocker. Two safe reproductions show plugin-controlled Git symlinks can cause host-readable content to be staged before Docker isolation: (1) a member evals/evals.* symlink is accepted through find_eval_file(...).exists(), parsed, and copied into task inputs before any symlink validation; (2) a repo-root skills or rules symlink is resolved before containment, making the external target the trusted root.

This is a pre-sandbox host-file disclosure path, not only a race. Reject links/reparse points and mount crossings before resolution; use descriptor-anchored no-follow reads/copies for member datasets (new and legacy layouts); preserve the lexical clone-root boundary for canonical refs; and add regressions for both attack paths.

Secondary integrity issue: fail closed when the Git-origin slug is unavailable — the current fallback can mark a foreign reference as fully evaluated.

Comment thread src/skillevaluator/tier3/plugin_eval.py Outdated
Comment thread src/skillevaluator/tier3/plugin_eval.py Outdated
@rng1995

rng1995 commented Aug 31, 2026

Copy link
Copy Markdown
Collaborator

@chrisknvidia Gentle ping when you have a chance: there are still three unresolved review threads on this PR. Please take a look and update the branch or reply on the threads where you disagree.

Signed-off-by: Narendran Raghavan <nraghavan@nvidia.com>
@rng1995

rng1995 commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Updated in f3aefb9: merged current main and resolved the conflicts, hardened provenance/canonical-ref/member-dataset staging, and fixed the custom-only sum-of-parts report path uncovered during the merge review. All addressed review threads are resolved. Local verification covered the full suite and focused security/report regressions; Ruff and git diff --check pass. All GitHub checks, including Python 3.12/3.13, DCO, CodeQL, packaging, Windows Tier 2, and macOS Tier 3, are green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants